Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added gasnet documentation #6382

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ct-clmsn
Copy link
Contributor

@ct-clmsn ct-clmsn commented Nov 8, 2023

Fixes: not applicable

Proposed Changes

  • adds gasnet documentation

Any background context you want to provide?

The GASNet PR didn't include the minimal documentation, this resolves that issue.

@hkaiser
Copy link
Member

hkaiser commented Nov 8, 2023

@ct-clmsn how does this integrate with the existing docs. Could you talk to @dimitraka about this? She can help with everything related to docs.

@ct-clmsn
Copy link
Contributor Author

ct-clmsn commented Nov 8, 2023

@hkaiser this mirrors the LCI documentation, so it explains the compile arguments. Will contact @dimitraka for details!

@dimitraka
Copy link
Contributor

dimitraka commented Nov 9, 2023

@ct-clmsn I suggest building the documentation. It's the only way of seeing how your docs look in html and check that everything is fine.

Some minors that I see at the moment:

  • First of all, for your doc to appear in the manual, you need to list it in the file https://github.com/STEllAR-GROUP/hpx/blob/master/docs/sphinx/manual.rst. So, you need to add manual/using_the_gasnet_parcelport in the correct place (probably under manual/using_the_lci_parcelport)
  • Secondly, as a general rule we want the title underlines to be at least as long as the title, otherwise sphinx will complain and this will add up to the documentation warnings. Just add more = to your titles.

@ct-clmsn
Copy link
Contributor Author

ct-clmsn commented Nov 9, 2023

@dimitraka updated the files, hope these align correctly!

@dimitraka
Copy link
Contributor

@ct-clmsn Did you also build the documentation? To do so I suggest using the docker image and building hpx with
-DHPX_WITH_DOCUMENTATION=ON (and whatever other flags you usually use). Then you just do make docs to build your docs

@dimitraka
Copy link
Contributor

@hkaiser @ct-clmsn Just letting you know that I built the documentation and it looks fine, so we can proceed with this one!

@ct-clmsn
Copy link
Contributor Author

@dimitraka thanks for verifying!

Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.11%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (76bcb86) 206733 176197 85.23%
Head commit (7c56d8f) 190772 (-15961) 162379 (-13818) 85.12% (-0.11%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#6382) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation

@hkaiser
Copy link
Member

hkaiser commented Feb 23, 2024

@dimitraka, @ct-clmsn is this good to go?

Copy link
Member

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants